Closed
Bug 1371511
Opened 8 years ago
Closed 8 years ago
InvalidArrayIndex_CRASH {@ mozilla::a11y::NotificationController::WillRefresh]
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox54 | --- | wontfix |
firefox55 | --- | fixed |
firefox56 | --- | fixed |
People
(Reporter: tsmith, Assigned: eeejay)
References
(Blocks 1 open bug)
Details
(4 keywords, Whiteboard: [fuzzblocker][adv-main55+][post-critsmash-triage])
Attachments
(1 file)
182 bytes,
text/html
|
Details |
This was found on Ubuntu 16.04 with m-c 20170606181015 6cd0639e02
I assume this is a safe crash but I'm not 100% so I've marked it s-s.
This test case requires the fuzzPriv extension to reproduce.
==23810==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x00000051b5b6 bp 0x7ffee27087f0 sp 0x7ffee2708680 T0)
#0 0x51b5b5 in MOZ_CrashPrintf src/mfbt/Assertions.cpp:63:3
#1 0x7f506af3b02f in InvalidArrayIndex_CRASH(unsigned long, unsigned long) src/xpcom/ds/nsTArray.cpp:26:3
#2 0x7f50741205d6 in ElementAt src/obj-firefox/dist/include/nsTArray.h:1048:7
#3 0x7f50741205d6 in operator[] src/obj-firefox/dist/include/nsTArray.h:1086
#4 0x7f50741205d6 in mozilla::a11y::NotificationController::WillRefresh(mozilla::TimeStamp) src/accessible/base/NotificationController.cpp:738
#5 0x7f50718f0045 in nsRefreshDriver::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:1791:12
#6 0x7f50718fea85 in mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:301:7
#7 0x7f50718fe742 in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:322:5
#8 0x7f5071900e3b in RunRefreshDrivers src/layout/base/nsRefreshDriver.cpp:754:5
#9 0x7f5071900e3b in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:667
#10 0x7f50718fc147 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run() src/layout/base/nsRefreshDriver.cpp:513:20
#11 0x7f506b01141e in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1321:14
#12 0x7f506b01d858 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:472:10
#13 0x7f506bde9c91 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:96:21
#14 0x7f506bd46d90 in RunInternal src/ipc/chromium/src/base/message_loop.cc:238:10
#15 0x7f506bd46d90 in RunHandler src/ipc/chromium/src/base/message_loop.cc:231
#16 0x7f506bd46d90 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:211
#17 0x7f507125d68f in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:156:27
#18 0x7f507491dce1 in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:283:30
#19 0x7f5074aee334 in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:4569:22
#20 0x7f5074aefea0 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4749:8
#21 0x7f5074af11f1 in XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4844:21
#22 0x4eb5a3 in do_main src/browser/app/nsBrowserApp.cpp:236:22
#23 0x4eb5a3 in main src/browser/app/nsBrowserApp.cpp:309
#24 0x7f50869a582f in __libc_start_main /build/glibc-9tT8Do/glibc-2.23/csu/../csu/libc-start.c:291
#25 0x41d0f8 in _start (m-c-1496772615-asan-opt/firefox+0x41d0f8)
Comment 1•8 years ago
|
||
is there any guidance I can follow to install fuzzPriv extension on my local build?
Reporter | ||
Comment 2•8 years ago
|
||
The plugin xpi can be found here: https://www.squarefree.com/extensions/domFuzzLite3.xpi
You can also do it by copying the contents of this extension directory[1] in to <firefox_profile_dir>/extensions/domfuzz@squarefree.com/
You can also use ffpuppet[2] to simplify launching the browser with a fresh temporary profile and cleaning up after. It can install the extension automatically. I'd recommend using ffpuppet because it prevents breaking your existing profile when using custom prefs.js and extensions. I use ffpuppet and so does our automation.
[1] https://github.com/MozillaSecurity/domfuzz/tree/master/dom/extension
[2] https://github.com/MozillaSecurity/ffpuppet
Comment 3•8 years ago
|
||
This is a safe crash, but I'm a little concerned that this code crashes so easily.
Keywords: csectype-bounds,
sec-moderate
Comment 4•8 years ago
|
||
here's a stack of the error:
1) handling child document fails to bind and gets shutdown [1]
2) then MaybeShutdownAccService shutdowns the accessibility service [2], I assume because GC collected all stuff and there's no reason to keep a11y running
3) we've got shutdown on the run -> crash
An easy fix would be to add a mDocument null check after each child document shutdown.
On the other hand I'm not 100% sure there's nothing bogus here. So that if that was the garbage collector who's cleared all stuff, then I would expect the total shutdown happens having GC stuff on the stack, but we have WillRefresh which is in a good state (mDocument is not null), which indicates that a11y is still running.
[1] https://dxr.mozilla.org/mozilla-central/source/accessible/base/NotificationController.cpp#763
[2] https://dxr.mozilla.org/mozilla-central/source/accessible/base/nsAccessibilityService.cpp?q=MaybeShutdownAccService&redirect_type=direct#1826
Comment 5•8 years ago
|
||
Ok, here's no-GC-on-stack issue explanation. GC kills XPCOM's accessibility service, it schedules a11y shutdown in 100ms [1], and then WillRefresh triggers right before the scheduled shutdown. Not yet clear, why child document fails to bind, since technically nothing shutdowns accessibility yet.
[1] https://dxr.mozilla.org/mozilla-central/source/accessible/xpcom/xpcAccessibilityService.cpp?q=mShutdownTimer-%3EInitWithFuncCallback%28ShutdownCallback%2C+this%2C+100%2C&redirect_type=single#82
Comment 6•8 years ago
|
||
cc'ing Eithan for ideas.
Assignee | ||
Comment 7•8 years ago
|
||
I think my fix in bug 1330765 may remedy this too..
Reporter | ||
Comment 8•8 years ago
|
||
This appears to be fixed, I haven't seen it since the patch in bug 1330765 landed.
How should we close this one? as a dup?
Flags: needinfo?(eitan)
Assignee | ||
Comment 9•8 years ago
|
||
Yup, makes sense. If this pops up again, open a new bug.
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(eitan)
Resolution: --- → DUPLICATE
Comment 10•8 years ago
|
||
sorry for being nitpicky here, but it doesn't seem a dupe, at least because they have different stack traces. I'm good to call it fixed if you cannot reproduce it any longer, however it's not that obvious with me how bug 1330765 helps it.
Resolution: DUPLICATE → FIXED
Comment 11•8 years ago
|
||
It would be good to get some consensus here just to make sure we're not missing anything. Can you please expand on why bug 1330765 may have solved this?
Flags: needinfo?(eitan)
Updated•8 years ago
|
Group: dom-core-security → core-security-release
Assignee | ||
Comment 12•8 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #11)
> It would be good to get some consensus here just to make sure we're not
> missing anything. Can you please expand on why bug 1330765 may have solved
> this?
The case seems to be similar: sub-documents loaded while starting and stopping a11y.
Also, I am only able to reproduce this crash when I revert the patch from bug 1330765.
Flags: needinfo?(eitan)
Updated•8 years ago
|
Assignee: nobody → eitan
status-firefox54:
--- → wontfix
status-firefox55:
--- → fixed
status-firefox56:
--- → fixed
status-firefox-esr52:
--- → unaffected
Depends on: 1330765
Target Milestone: --- → mozilla56
Updated•8 years ago
|
Whiteboard: [fuzzblocker] → [fuzzblocker][adv-main55+]
Updated•8 years ago
|
Flags: qe-verify-
Whiteboard: [fuzzblocker][adv-main55+] → [fuzzblocker][adv-main55+][post-critsmash-triage]
Updated•7 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•